[BUZZOK-31834] Decode versions in order to install key dependencies first - #896
Conversation
|
🎫 Jira: |
Code OwnershipCli Maintainers
Review requested from the teams above. Labels will be removed automatically upon approval. |
chasdr
left a comment
chasdr
left a comment
There was a problem hiding this comment.
lgtm 👍 traced the order-preservation through GetRequirements -> CheckPrerequisiteList into MissingTools, install order matches the file end to end, and the regression test defeats both sort and coincidence.
@ajalon1 @taras-pokornyy tagging you two for a second look, you both know the dependency install path better than i do. anything off about walking yaml.Node for order here?
Confirmed |
|
@taras-pokornyy good catch on duplicate key. Actually it was not last wins, Yaml schema v3 would fail on it when deconding into a map, and my implementation didn't. Added a fix and a test. |
3 participants
RATIONALE
I want to add
pulumi-datarobottoversions.yamlto unify all the dependencies, and move away from a brittle fixes ininfra/Taskfile.yaml. But when I tested it on a clean system,dr dependency installfailed because there was nopulumibecause it does not respect the order of dependencies installation in the YAML (which is expected for a random map). This updates the decode to iterate over config as it is defined.CHANGES
PR Automation
Comment-Commands: Trigger CI by commenting on the PR:
/trigger-smoke-testor/trigger-test-smoke- Run smoke tests/trigger-install-testor/trigger-test-install- Run installation testsLabels: Apply labels to trigger workflows:
run-smoke-testsorgo- Run smoke tests on demand (only works for non-forked PRs)Important
For Forked PRs: The
run-smoke-testslabel won't work. A required Smoke Tests check will block merge until a maintainer acts:/approve-smoke-teststo run smoke tests (results will set the check)/skip-smoke-teststo bypass the check without running testsPlease comment requesting a maintainer review if you need smoke tests to run.
Note
Low Risk
Localized change to YAML parsing in the tools package; install order becomes deterministic and matches the YAML, with a regression test and no auth or data-handling impact.
Overview
GetRequirementsFromDirno longer unmarshalsversions.yamlinto a map, which made prerequisite order nondeterministic and could rundr dependency installsteps before their dependencies (e.g.pulumi-datarobotbeforepulumi). It now decodes withyaml.Nodeand walks top-level keys in file order, with checks for an empty document and a top-level mapping.A regression test runs
GetRequirementsFromDirmany times and asserts keys stay in declaration order (tool-z,tool-a,tool-m).Reviewed by Cursor Bugbot for commit 6dce629. Configure here.